windows: integrate Windows into shared CI matrices (R8, depends on #2422) - #2425

Review: Windows R8-on-R7 integration

Reviewed the R8-only range (09a23ef..f293677, 84 files) — the actual scope of this PR per the description; the full base-diff also carries the stacked #2422 work, which is out of scope here.

This is a large, well-executed portability change: CI matrix consolidation (removing windows.yml), platform-split demos, portable LLDB tooling, and Windows PE metrics. Test coverage is strong — nearly every production change is paired with a test (TestAddPESections, TestWindowsTarXzTools, TestWindowsDebugPointerParameter, TestRunQuietIncludesCommandOutput, etc.). Comments explain the Windows-specific rationale well. No high-severity or blocking issues found; the notes below are refinements.

Cross-cutting notes (no reliable single inline location):

• Supply-chain — github.com/cpunion/lib replacement. The root go.mod:30 and all demo/tool go.mod files replace github.com/goplus/lib with a personal fork pinned to v0.0.0-20260822071805-ebe22363a225. The PR body already flags this as temporary, which is right — the important one is the root go.mod (the compiler build itself), not just the demos. go.sum hashes guard against tampered fetches, but this should revert to an upstream-owned tag before release. Tracking this is worthwhile.
• internal/crosscompile/fetch.go — .tar.xz extraction trust path. The Go-native extractTarGz/extractZip paths enforce a path-traversal (illegal file path) guard; the .tar.xz path shells out to external tar/bsdtar/MSYS2 tar with --force-local and relies on the tool's own protections. Combined with downloads still having no checksum/signature verification (downloadFile), a moved/altered release archive could write outside dest. This is largely pre-existing (the PR only changed tool selection), but pinning checksums for the ESP/WASI archives would close the gap.
• _demo/c/syncdebug/once_default.go vs once_windows.go. The two files implement the same llsync.Once test with gratuitously different structure (package-level vars + named funcs vs. locals + closures). If the closure form works on Windows it almost certainly works everywhere; a shared implementation (or a comment explaining why they must differ) would reduce drift risk.

Verified and dismissed: an internal reviewer flagged test_windows_runtime.ps1 referencing a missing check_windows_imports.ps1, but that file exists at .github/workflows/check_windows_imports.ps1 — not an issue.

Review: R8 shared-CI Windows integration

Reviewed the locked R8-only range e1791699...f2936775 (357 files, ~13.7k/-2.2k). Focus was the production runtime/codegen surface — Windows fault handling, SEH unwind, semaphores, threading, RNG, print, setjmp, and the CI matrix reshaping. Four review passes ran (quality, performance, security, docs).

Overall: high-quality, carefully engineered work. The _default/_windows/_windows_<arch> build-tag partitioning is, in nearly every family I checked (setjmp, fault, cpuprof, sema, foreign-thread, ffi-abi, gc-threads, llvm-config), provably complete and non-overlapping, and the native C/asm bridges (rand, fault handler, print, syscall, setjmp, thread/FLS) are bounds-checked with no memory-safety, injection, or weak-randomness issues found. CI trust boundaries are sound: pull_request (not pull_request_target), contents: read, secrets confined to appropriate steps, toolchain pins consistent (Go 1.26.7 / LLVM 19 / VS2022).

A few items worth a look, left as inline comments. None are blocking.

Notes (not inline):

• Supply-chain (Medium): replace github.com/goplus/lib => github.com/cpunion/lib is added to the root and every sub-module go.mod (_xtool, doc/_readme, _demo/{c,go,py,embed}). Content is pinned + go.sum-verified, and the PR body flags it as temporary — recommend gating release on moving these commits to an org-owned/tagged upstream so a released version never depends on a personal fork. (Inline note on root go.mod.)
• CI hardening (Low, pre-existing): .github/workflows/benchmark.yml interpolates ${{ github.event.pull_request.base.ref }} directly into a run: block. The line itself is pre-existing (this PR only adds shell: bash to that step), exploitability is low (base ref is an existing branch name, pull_request trigger, contents: read, no secrets), but normalizing to the env:-indirection pattern already used elsewhere in the same file would be good hygiene.

Verification of the described validation matrix (287 packages, ./test/go, coverage %, LLDB suite, cross-target runtime builds) relies on the CI results reported as green; I reviewed the code and configuration, not the live job logs.

Additional findings

• runtime/internal/lib/runtime/runtime_windows.go:1: [P2] Windows build-tag overlaps baremetal/wasm (duplicate LLGoFiles): runtime_windows.go is tagged //go:build windows, while sibling runtime_baremetal.go is //go:build baremetal || wasm. A windows && (baremetal || wasm) build satisfies both, and both declare LLGoPackage/LLGoFiles constants — that configuration would fail with duplicate declarations. runtime_default.go in the same package correctly excludes windows (!baremetal && !wasm && !windows), so this is the one file in the family whose tag isn't provably disjoint. If windows && baremetal/windows && wasm is not a real target the impact is latent, but tightening to //go:build windows && !baremetal && !wasm removes the ambiguity at zero cost.

FennoAI Review — Windows R8-on-R7 integration

Large, well-structured PR. Build-tag splits (*_windows.go / *_default.go / *_unix.go), the crosscompile Windows toolchain selection, the MSVC import-library resolver, and the sync/once simplification are clean and well-covered by new tests. No blocking correctness regressions found. A few items worth confirming before merge, plus inline notes.

Items without a single reliable inline location:

• Fork dependency in mainline modules. Every go.mod (root, _demo/*, _xtool, doc/_readme) adds replace github.com/goplus/lib => github.com/cpunion/lib v0.0.0-20260827173821-.... This redirects the whole C/OS binding surface (sockets, os, net) to a personal fork. go.sum pins it so it isn't silently mutable, but please confirm this is a temporary bootstrap to be reverted to a tagged upstream goplus/lib release before merge. Some go.sum files also retain an older unused pseudo-version (...20260822071805-...) that go mod tidy would drop.
• .github/workflows/benchmark.yml (~L47–51): git fetch https://github.com/${{ github.event.pull_request.base.repo.full_name }}.git ${{ github.event.pull_request.base.ref }} interpolates PR metadata directly into a run: block. These are maintainer-controlled target-branch values (low risk) and the PR only added shell: bash here, but passing them via env: and quoting "$VAR" is the safer pattern.
• Downloaded toolchain archives lack checksum/signature verification (internal/crosscompile/fetch.go downloadFile). The PR adds a new Windows source (github.com/espressif/llvm-project/releases/...) whose archive is extracted then executed as the compiler. Pin-by-tag helps but recording/verifying a SHA-256 would close the gap — especially combined with the extractZip note below.

Findings without inline locations

• internal/crosscompile/fetch.go:362: extractZip writes to filepath.Join(dest, file.Name) with no containment check, so a crafted entry like ../../evil can escape dest (zip-slip). The sibling extractTarGz already guards this (illegal file path check). Current sources are trusted/HTTPS so exploitability is limited, but since this file is being touched for Windows it's worth adding the same prefix check (plus a symlink guard) here.
• README.md:356: The install guide still reads ### on Windows / TODO, but this PR makes Windows a CI-tested target — doc.yml even runs the README code blocks on windows-2022, so the docs are exercised on Windows while telling users installation is unimplemented. Please fill this in (mirroring the CI setup-deps steps) or mark it experimental. The support matrix around L50–56 also omits the new Windows amd64 native CI coverage.

Re-review: R8 diff at 29a713b47 (post-#2422)

Re-reviewed the locked range ff9bad85...29a713b4 (128 files, +2550/-775). Now that dependency #2422 has merged, this is a genuinely R8-only diff — much smaller and cleaner than the earlier stacked view. Ran all four passes (quality, performance, security, docs).

Prior findings — status:

• Build-tag overlap (runtime_windows.go) and semaRelease wake-per-release — both now land at the merged base via #2422 and are outside this PR's diff range. Not re-flagged here; they belong to #2422's history.
• goplus/lib → cpunion/lib replace — still present and re-introduced by this PR (see below).

New results this pass:

• Quality: No in-range findings. Spot-verified correct: the _default/_unix/_windows sync splits, macho_size.go close-before-rename handling on Windows, clang.go CommandLineToArgvW backslash quoting, and ssa/eh.go arch-dispatched setjmp/longjmp.
• Performance: No regressions. The crosscompile archiving change (long argv → objects.rsp response file for llvm-ar) is a scalability improvement for the many-object newlib case; the Windows GNU tar/xz path is a large win; resolveMSVCImportLibraries is bounded one-time link-step cost gated behind a fast-path check that leaves non-MSVC builds untouched.
• Security: CI trust boundaries are sound — pull_request (not pull_request_target), persist-credentials: false on untrusted checkouts, dispatch/token jobs gated to non-PR events on xgo-dev/llgo with ref/SHA validation, and the native C shims (profile_windows.c, sync_windows.c) are bounds-checked with overflow-aware deadline math. One finding, inline below.
• Docs: No findings. Renamed llvm_config_windows.go comment matches the new name/behavior; toolchain pins (Go 1.26.7 / LLVM 19 / VS2022) are internally consistent.

I reviewed code and configuration, not the live CI logs — the reported green matrix and 97.98% patch coverage are taken as stated.